Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass additional options to JVM (WIP) #505

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

willmostly
Copy link
Contributor

Description

Allow passing additional java options to the startup command

Additional context and related issues

Users may need to set additional java options, e.g. using -Djavax.net.ssl.truststore=/path/to/truststore in combination with a volume for mounting a certificate bundle.

Release notes

  • ( ) This is not user-visible or is docs only, and no release notes are
    required.
  • ( ) Release notes are required. Please propose a release note for me.
  • ( x) Release notes are required, with the following suggested text:

Helm Chart

Allow passing additional java options to the startup command

@cla-bot cla-bot bot added the cla-signed label Oct 2, 2024
- "/usr/lib/trino/gateway-ha-jar-with-dependencies.jar"
- "/etc/gateway/config.yaml"
- java
{{- toYaml append .Values.javaOptions list "-XX:MinRAMPercentage=80.0" "-XX:MaxRAMPercentage=80.0" "-jar" "/usr/lib/trino/gateway-ha-jar-with-dependencies.jar" "/etc/gateway/config.yaml"}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about moving the options all into the values file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this setup you cant really override those values right? Or at least they would show up twice..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that could be nice and simple, i'll update this

@willmostly willmostly changed the title Pass additional options to JVM Pass additional options to JVM (WIP) Oct 3, 2024
@@ -52,6 +52,15 @@ config:
managedApps:
- io.trino.gateway.ha.clustermonitor.ActiveClusterMonitor

# Startup command for Trino Gateway process. Add additional java options here if required
command:
- "java"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two space indent like in below or four like above.. maybe we should make the whole file consistent

@mosabua
Copy link
Member

mosabua commented Oct 3, 2024

If we go with this approach where the whole command is configurable.. which is fine by me .. we should update commit message, PR name, release notes suggestion, and also add relevant info to the docs.

Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See earlier comment of mine...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants